feat(upgrade): add verification release is deployable #3136

JGAntunes · 2025-10-29T19:18:30Z

What this PR does / why we need it:

Adds verifications that the upgrade is possible.

Which issue(s) this PR fixes:

https://app.shortcut.com/replicated/story/130790/add-verification-that-release-is-deployable

Does this PR require a test?

Yes, added unit tests, we don't have a dryrun test for upgrades yet, I would imagine this would be one test we could add. Unsure if we should make it part of this PR or not.

~~There's still some manual tests I want to run though.~~

Does this PR require a release note?

NONE

Does this PR require documentation?

NONE

github-actions · 2025-10-30T13:14:32Z

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-959f483" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-959f483?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

emosbaugh · 2025-10-30T13:20:57Z

pkg-new/validation/upgradable.go

+		if len(requiredReleases) > 0 {
+			// Extract version labels from required releases
+			for _, release := range requiredReleases {
+				requiredVersions = append(requiredVersions, release.VersionLabel)


why dont you check TargetAppSequence here like you do for online?

This is wrong yes, I'm trying to figure out how the required versions are sorted and what gets added to the metadata.

Addressed it in the latest changes.

pkg-new/validation/upgradable.go

emosbaugh · 2025-10-30T13:28:32Z

pkg-new/validation/upgradable.go

+	}
+
+	// Check if minor version is being skipped
+	if targetK8s.Minor() > currentK8s.Minor()+1 {


do you also need to check for downgrade here?

I'm not doing it because we're also checking for EC version downgrades already (and block it altogether) so that code path would never be used. We could do it if we want to ensure correctness though.

Ah I completely forgot about the fact you could theoretically still move to a higher EC version but a lower kube version. You're right, sorry I missed it @emosbaugh. Fixed via - 959f483

emosbaugh · 2025-10-30T13:32:26Z

pkg-new/replicatedapi/client.go

+}
+
+// GetPendingReleases fetches pending releases from the Replicated API
+func (c *client) GetPendingReleases(ctx context.Context, channelID string, currentSequence int64, opts *PendingReleasesOptions) (*PendingReleasesResponse, error) {


is it worth adding reporting info to this call? i know we can add the app status, sequence, number of nodes, etc...

it could be done in a follow up

Good question... Idk 🤷 who would be the best person to talk about this reporting info, its use and what's expected?

We have other instances in the cli where we hit this endpoint without sending any data whatsoever -

embedded-cluster/cmd/installer/cli/release.go

Lines 29 to 36 in 030261b

func getCurrentAppChannelRelease(ctx context.Context, license *kotsv1beta1.License, channelID string) (*apiChannelRelease, error) {

query := url.Values{}

query.Set("selectedChannelId", channelID)

query.Set("channelSequence", "") // sending an empty string will return the latest channel release

query.Set("isSemverSupported", "true")

apiURL := replicatedAppURL()

url := fmt.Sprintf("%s/release/%s/pending?%s", apiURL, license.Spec.AppSlug, query.Encode())

cmd/installer/cli/upgrade.go

emosbaugh · 2025-10-30T13:44:19Z

cmd/installer/cli/upgrade.go

+		}
+		upgradeConfig.license = updatedLicense
+		upgradeConfig.licenseBytes = licenseBytes
+		upgradeConfig.replicatedAPIClient = replicatedAPI


this should be taken out of the conditional as it is only set in airgap mode

It's the other way around, it's only set in online mode right? And isn't that what we want? In airgap there's no point in initialising the replicated API client right?

Co-authored-by: Ethan Mosbaugh <[email protected]>

sgalsaleh · 2025-11-04T18:24:47Z

cmd/installer/cli/upgrade.go

+	}
+
+	// Perform validation
+	if err := validation.ValidateIsReleaseUpgradable(ctx, opts); err != nil {


the logic that builds these opts should also be put into its own "hop" (sub-function) and unit tested IMO because there's an external dependency at the end of this path. note: the installation object should be passed to it though so it doesn't interact with the cluster or need a fake k8s client.

@sgalsaleh, we cannot unit test this method, it has a replicated API dependency. Internally we've added a function that we unit test:

embedded-cluster/cmd/installer/cli/upgrade.go

Lines 630 to 633 in 041d8ec

} else {

if err := opts.WithOnlineRequiredReleases(ctx, upgradeConfig.replicatedAPIClient); err != nil {

return fmt.Errorf("failed to extract required releases from replidated API's pending release call: %w", err)

}

This method is unit tested:

embedded-cluster/cmd/installer/cli/upgrade.go

Lines 627 to 629 in 041d8ec

if err := opts.WithAirgapRequiredReleases(upgradeConfig.airgapMetadata); err != nil {

return fmt.Errorf("failed to extract required releases from airgap metadata: %w", err)

}

So I'm trying to understand what we need to unit test, is it this logic?

embedded-cluster/cmd/installer/cli/upgrade.go

Lines 600 to 623 in 041d8ec

// Get current and target EC/K8s versions

var currentECVersion string

if currentInstallation.Spec.Config != nil {

currentECVersion = currentInstallation.Spec.Config.Version

}

targetECVersion := versions.Version

// Build validation options

opts := validation.UpgradableOptions{

CurrentECVersion: currentECVersion,

TargetECVersion: targetECVersion,

License: upgradeConfig.license,

}

// Add current app version info if available

if upgradeConfig.currentAppVersion != nil {

opts.CurrentAppVersion = upgradeConfig.currentAppVersion.VersionLabel

opts.CurrentAppSequence = upgradeConfig.currentAppVersion.ChannelSequence

}

// Add target app version info

opts.TargetAppVersion = channelRelease.VersionLabel

opts.TargetAppSequence = channelRelease.ChannelSequence

Or we only add a test for airgap?

cursor · 2025-11-05T12:37:17Z

pkg-new/replicatedapi/client.go

+
+	params := url.Values{}
+	params.Set("selectedChannelId", channelID)
+	params.Set("channelSequence", fmt.Sprintf("%d", currentSequence))


Bug: Mismatched Query Parameter Name in API Call

Query parameter name mismatch: the code sets "channelSequence" but the tests expect "currentSequence". Line 185 uses params.Set("channelSequence", ...) but the test assertions at client_test.go lines 417, etc. expect r.URL.Query().Get("currentSequence"). This will cause the API call to fail or return incorrect results because the server expects "currentSequence" as the parameter name.

cursor · 2025-11-05T12:37:17Z

cmd/installer/cli/upgrade.go

+		}
+	} else {
+		if err := opts.WithOnlineRequiredReleases(ctx, upgradeConfig.replicatedAPIClient); err != nil {
+			return fmt.Errorf("failed to extract required releases from replidated API's pending release call: %w", err)


Bug: Typo in API error: “replidated” should be “replicated”

Typo in error message: "replidated" should be "replicated". The error message reads "failed to extract required releases from replidated API's pending release call" but it should be "replicated".

cursor · 2025-11-05T13:10:44Z

cmd/installer/cli/upgrade.go

+	var currentECVersion string
+	if currentInstallation.Spec.Config != nil {
+		currentECVersion = currentInstallation.Spec.Config.Version
+	}


Bug: Invalid Version Validation When Config Is Nil

When currentInstallation.Spec.Config is nil, currentECVersion will be an empty string. This empty string is then passed to validateECVersionDowngrade() and validateK8sVersion() which call semver.NewVersion(""), causing a parsing error rather than a ValidationError. This breaks the error handling logic at lines 372-377 which expects ValidationError types for user-facing validation failures. The code should skip EC version and K8s version validation when currentECVersion is empty, or handle the empty case appropriately.

JGAntunes · 2025-11-05T13:11:20Z

pkg-new/validation/errors.go

+// NewRequiredReleasesError creates a ValidationError indicating that intermediate
+// required releases must be installed before upgrading to the target version
+func NewRequiredReleasesError(requiredVersions []string, targetVersion string) *ValidationError {
+	return &ValidationError{
+		Message: fmt.Sprintf("this upgrade requires installing intermediate version(s) first: %s. Please go through this upgrade path before upgrading to %s",
+			strings.Join(requiredVersions, ", "), targetVersion),
+	}
+}
+
+// NewAppVersionDowngradeError creates a ValidationError indicating that the target
+// app version is older than the current version
+func NewAppVersionDowngradeError(currentVersion, targetVersion string) *ValidationError {
+	return &ValidationError{
+		Message: fmt.Sprintf("downgrade detected: cannot upgrade from app version %s to older version %s", currentVersion, targetVersion),
+	}
+}
+
+// NewECVersionDowngradeError creates a ValidationError indicating that the target
+// Embedded Cluster version is older than the current version
+func NewECVersionDowngradeError(currentVersion, targetVersion string) *ValidationError {
+	return &ValidationError{
+		Message: fmt.Sprintf("downgrade detected: cannot upgrade from Embedded Cluster version %s to older version %s", currentVersion, targetVersion),
+	}
+}
+
+// NewK8sVersionSkipError creates a ValidationError indicating that the Kubernetes
+// version upgrade skips a minor version, which is not supported by Kubernetes
+func NewK8sVersionSkipError(currentVersion, targetVersion string) *ValidationError {
+	return &ValidationError{
+		Message: fmt.Sprintf("Kubernetes version skip detected: cannot upgrade from k8s %s to %s. Kubernetes only supports upgrading by one minor version at a time",
+			currentVersion, targetVersion),
+	}
+}
+
+// NewK8sVersionDowngrade creates a ValidationError indicating that the Kubernetes
+// version upgrade downgrades the kubernetes version used, which is not supported
+func NewK8sVersionDowngrade(currentVersion, targetVersion string) *ValidationError {
+	return &ValidationError{
+		Message: fmt.Sprintf("Kubernetes version downgrade detected: cannot downgrade from k8s %s to %s. Kubernetes downgrades are not supported",
+			currentVersion, targetVersion),
+	}
+}


@ajp-io let me know what you think of the copy of these errors, there's some examples in the description of the PR on how these show up in the CLI.

sgalsaleh · 2025-11-05T16:32:34Z

Merging as failing test is unrelated.

JGAntunes changed the title ~~Jgantunes/sc 130790/add verification that release is deployable~~ feat(upgrade): add verification release is deployable Oct 29, 2025

JGAntunes self-assigned this Oct 30, 2025

github-actions bot added the type::feature label Oct 30, 2025

JGAntunes force-pushed the jgantunes/sc-130790/add-verification-that-release-is-deployable branch from 5e75ed0 to e21fcad Compare October 30, 2025 13:26

emosbaugh requested changes Oct 30, 2025

View reviewed changes

JGAntunes and others added 3 commits November 4, 2025 11:39

feat(upgrade): add verification release is deployable

012b892

chore: tests and cleaning up code

e6d0c86

Apply suggestions from code review

2fabba8

Co-authored-by: Ethan Mosbaugh <[email protected]>

JGAntunes force-pushed the jgantunes/sc-130790/add-verification-that-release-is-deployable branch from 0f083d4 to 2fabba8 Compare November 4, 2025 11:40

JGAntunes and others added 3 commits November 4, 2025 17:24

chore: address feedback and fix airgap required version setup

df62f74

Update pkg-new/validation/upgradable.go

0249e25

Co-authored-by: Ethan Mosbaugh <[email protected]>

chore: more comments and tests

041d8ec

JGAntunes marked this pull request as ready for review November 4, 2025 18:17

JGAntunes requested a review from emosbaugh November 4, 2025 18:18

This comment was marked as outdated.

Sign in to view

chore: feedback

473d223

sgalsaleh reviewed Nov 4, 2025

View reviewed changes

fix: client param naming

1ab3345

cursor bot reviewed Nov 5, 2025

View reviewed changes

JGAntunes added 2 commits November 5, 2025 12:53

fix: client tests

2ad6cd3

fix: also validate kuberntes downgrades

959f483

cursor bot reviewed Nov 5, 2025

View reviewed changes

JGAntunes commented Nov 5, 2025

View reviewed changes

JGAntunes requested a review from ajp-io November 5, 2025 13:19

sgalsaleh approved these changes Nov 5, 2025

View reviewed changes

sgalsaleh merged commit b4e73be into main Nov 5, 2025
49 of 51 checks passed

sgalsaleh deleted the jgantunes/sc-130790/add-verification-that-release-is-deployable branch November 5, 2025 16:32

	func getCurrentAppChannelRelease(ctx context.Context, license kotsv1beta1.License, channelID string) (apiChannelRelease, error) {
	query := url.Values{}
	query.Set("selectedChannelId", channelID)
	query.Set("channelSequence", "") // sending an empty string will return the latest channel release
	query.Set("isSemverSupported", "true")

	apiURL := replicatedAppURL()
	url := fmt.Sprintf("%s/release/%s/pending?%s", apiURL, license.Spec.AppSlug, query.Encode())

	} else {
	if err := opts.WithOnlineRequiredReleases(ctx, upgradeConfig.replicatedAPIClient); err != nil {
	return fmt.Errorf("failed to extract required releases from replidated API's pending release call: %w", err)
	}

	if err := opts.WithAirgapRequiredReleases(upgradeConfig.airgapMetadata); err != nil {
	return fmt.Errorf("failed to extract required releases from airgap metadata: %w", err)
	}

	// Get current and target EC/K8s versions
	var currentECVersion string
	if currentInstallation.Spec.Config != nil {
	currentECVersion = currentInstallation.Spec.Config.Version
	}

	targetECVersion := versions.Version

	// Build validation options
	opts := validation.UpgradableOptions{
	CurrentECVersion: currentECVersion,
	TargetECVersion: targetECVersion,
	License: upgradeConfig.license,
	}

	// Add current app version info if available
	if upgradeConfig.currentAppVersion != nil {
	opts.CurrentAppVersion = upgradeConfig.currentAppVersion.VersionLabel
	opts.CurrentAppSequence = upgradeConfig.currentAppVersion.ChannelSequence
	}

	// Add target app version info
	opts.TargetAppVersion = channelRelease.VersionLabel
	opts.TargetAppSequence = channelRelease.ChannelSequence

feat(upgrade): add verification release is deployable #3136

feat(upgrade): add verification release is deployable #3136

Conversation

JGAntunes commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR require a test?

Does this PR require a release note?

Does this PR require documentation?

Uh oh!

github-actions bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor bot Nov 5, 2025

Choose a reason for hiding this comment

Bug: Mismatched Query Parameter Name in API Call

Uh oh!

cursor bot Nov 5, 2025

Choose a reason for hiding this comment

Bug: Typo in API error: “replidated” should be “replicated”

Uh oh!

cursor bot Nov 5, 2025

Choose a reason for hiding this comment

Bug: Invalid Version Validation When Config Is Nil

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgalsaleh commented Nov 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JGAntunes commented Oct 29, 2025 •

edited

Loading

github-actions bot commented Oct 30, 2025 •

edited

Loading